[colorize_ansi] Remove the possibility to use anything else than a MessageStyle#8412
Conversation
Pierre-Sassoulas
added
Breaking changes for 3.0 🦤
Maintenance
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #8412 +/- ##
==========================================
+ Coverage 95.69% 95.71% +0.02%
==========================================
Files 175 175
Lines 18454 18451 -3
==========================================
+ Hits 17659 17660 +1
+ Misses 795 791 -4
|
| style: tuple[str, ...] = () | ||
| """Tuple of style strings (see `ANSI_COLORS` for available values).""" | ||
|
|
||
| def get_ansi_code(self) -> str: |
There was a problem hiding this comment.
Let's make this private
There was a problem hiding this comment.
It's a little complicated to make a public class function private (then we need to call it with .__MessageStyle_ ... I'm wondering if we could use colorama entirely instead. Or make this a public API, it's unlikely it's ever going away.
There was a problem hiding this comment.
I don't understand your comment. We can just make the function private and keep MessageStyle public right? We have similar patterns for all of our other classes: the class itself is public but not all method are public.
There was a problem hiding this comment.
>>> class MessageStyle:
... def __ansi_color(self):
... print("ansi")
...
>>> m = MessageStyle()
>>> m.__ansi_color()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: 'MessageStyle' object has no attribute '__ansi_color'
>>> m._MessageStyle__ansi_color()
ansiThere was a problem hiding this comment.
I meant _get_ansi_code. That signals that it is private and not part of MessageStyle's API
There was a problem hiding this comment.
Ha ok, I call that 'protected'.
There was a problem hiding this comment.
Well it could be made private with a small refactor f591319
This comment has been minimized.
This comment has been minimized.
Pierre-Sassoulas
force-pushed
the
colorize-ansi-deprecation
branch
from
690e59a to
64e5b95
Compare
|
🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉 This comment was generated for commit f591319 |
2 participants
Type of Changes
Description
I think we could be a lot more liberal with the breaking change for small internal things like that.